Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

I signed it!

CLAs look good, thanks!

Hey @vicb - is there any chance you could merge it into the lib soon? It is crucial to get it work with es6-shim. And in fact angular2 doesn't work either due to that issue.

I'm going to take a look today. Isn't the var you point to in the issue a local variable ?

And in fact angular2 doesn't work either due to that issue.

Could you expand on that. Ng2 tests pass down to IE9 and I can't see where es6-shim defines the global Symbol to be an empty object.

I am not opposed to merge this change to make the code more robust but I'd like to understand what your issue is first.

Probably tests in angular 2 are being run without any additional libraries (as es6-shim) and then pass because there is no problem in such a case. Screenshot below is taken on VM (Win 7, IE 11) and shows what the global Symbol can be with shim:

I'm not 100% sure but I guess this particular line of es6-shim makes it happen.

Regarding the condition here I think that checking like they did is much more robust:
https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L246 (Type.symbol is defined here).

It prevents situation like I met so that global Symbol is not really what we think it is and there is an assumption that can be run as a function/symbol. I made changes to check if it is function (so Symbol() doesn't produce an error) 'cause it's enough to not break the app and the only problem could be when someone names the global function Symbol and didn't mean the one from ES6.

Is it fair enough?

if Symbol is {} then it's a bug in es6-shim, right ?

Yes, you're right - it shouldn't be like this and that's another story. But on the other hand assumption that something which is defined (typeof Symbol !== 'undefined') can be invoked as Symbol() isn't enough, don't you think? This line makes the library more robust. That's why I opened this PR. Hope it makes sense for you.

The change makes sense but the issue doesn't and it is always better to understand the root causes of issues.

I've run the following test on IE11 (via Sauce Labs):

    fit('should test', function() {
      console.log('Promise = ', Promise);
      console.log('Symbol = ', Symbol);
    });

results:

## es6-shim 
$ karma start karma-sauce.conf.js 
WARN [watcher]: Pattern "/home/victor/dart/zone.js/examples/js/*.js" does not match any file.
INFO [framework.browserify]: 38360 bytes written (0.23 seconds)
INFO [framework.browserify]: bundle built
INFO [karma]: Karma v0.12.37 server started at http://localhost:9876/
INFO [launcher]: Starting browser internet explorer 11 (Windows 8.1) on SauceLabs
INFO [launcher.sauce]: internet explorer 11 (Windows 8.1) session at https://saucelabs.com/tests/dd46c71c39b9419fadb90669788b1570
INFO [IE 11.0.0 (Windows 8.1 0.0.0)]: Connected on socket 8SDmHm4lXWylm3THOEoq with id 21792575
LOG: 'Promise = ', function Promise(resolver) { ... }
LOG: 'Promise = ', function Promise(resolver) { ... }
IE 11.0.0 (Windows 8.1 0.0.0) Util Custom assertions should test FAILED
    ReferenceError: 'Symbol' is undefined
       at Anonymous function (/home/victor/dart/zone.js/test/util.spec.js:11:7)
IE 11.0.0 (Windows 8.1 0.0.0) Util Custom assertions should test FAILED
    ReferenceError: 'Symbol' is undefined
       at Anonymous function (/home/victor/dart/zone.js/test/util.spec.js:11:7)
IE 11.0.0 (Windows 8.1 0.0.0): Executed 1 of 63 (1 FAILED) (skipped 62) ERROR (0.06 secs / 0.002 secs)
victor@victor-n76:~/dart/zone.js [master $% u=]

## NO es6-shim 
$ karma start karma-sauce.conf.js 
WARN [watcher]: Pattern "/home/victor/dart/zone.js/examples/js/*.js" does not match any file.
INFO [framework.browserify]: 38360 bytes written (0.23 seconds)
INFO [framework.browserify]: bundle built
INFO [karma]: Karma v0.12.37 server started at http://localhost:9876/
INFO [launcher]: Starting browser internet explorer 11 (Windows 8.1) on SauceLabs
INFO [launcher.sauce]: internet explorer 11 (Windows 8.1) session at https://saucelabs.com/tests/d8a34cea43304c9892a445b4d0e24e16
INFO [IE 11.0.0 (Windows 8.1 0.0.0)]: Connected on socket vRadXsirY6gTIsSzOOB0 with id 15055070
IE 11.0.0 (Windows 8.1 0.0.0) Util Custom assertions should test FAILED
    ReferenceError: 'Promise' is undefined
       at Anonymous function (/home/victor/dart/zone.js/test/util.spec.js:10:7)
IE 11.0.0 (Windows 8.1 0.0.0) Util Custom assertions should test FAILED
    ReferenceError: 'Promise' is undefined
       at Anonymous function (/home/victor/dart/zone.js/test/util.spec.js:10:7)
IE 11.0.0 (Windows 8.1 0.0.0): Executed 1 of 61 (1 FAILED) (skipped 60) ERROR (0.062 secs / 0.003 secs)

As you can see es6-shim never set Symbol to an array (Promise is only here to make sure that es6-shim is included)

Ok. I've investigated problem once again and the issue is from both RxJS and es6-shim (RxJS creates it as object and then es6-shim applies iterator etc.). But my though was if there's possibility to avoid interference due to other libraries - why not to do this?

Sorry for not being precise with env - I haven't though it is so crucial for the problem this PR is trying to solve.

The thing is we had no clue what the underlying issue was. Could you pen a defect against RxJS (and link it here) ? Thanks

You can go to this plunker: http://plnkr.co/edit/ylqvcUi3dRcxVDB7MarZ?p=preview
And there's result on Win 7 / IE 11 (BrowserStack):

Could you remove es6-shim from this plunk and open an issue against the RxJS project ?

Yes, I'll definitely do so (even today).

And regarding zone.js - should I make checking in this PR more strict (as in es6-shim) or this version is enough for you guys?

Ok, here's the RxJS issue: ReactiveX/rxjs#988

Thanks for creating the issue.

I'll merge your PR as is

landed as 6714be6

🚀